Skip to content

Conversation

@JKHHai
Copy link

@JKHHai JKHHai commented Aug 27, 2018

Makes changes to the CoreIR library.

Main changes:

  • RemoveBulkConnections: Fixed a bug where inouts were not recognized as bit connections
  • addDefaultGenArgs: Fixed a bug where the default parameters were not being propagated to the Verilog writer.
  • Module: Made changes to Module constructor, so that Modules can be generated with empty genargs, for greater flexibility.

JKHHai added 4 commits August 24, 2018 17:30
- Modified Module Declaration so that modules can now be generated with empty parameter lists (when generators require no parameters). Added ASSERT statements to ensure that other exceptions are still caught.
- Modified RemoveBulkConnections pass so that a BitInOut type is recognized as a Bit Type.
Modified the Makefile of CoreIR with a Debug flag, in order to allow debugging with GDB.
Modified `verilog` pass of CoreIR so that the default generator parameter values are propagated to the Verilog writer.
Removed all unnecessary comments.
@JKHHai JKHHai changed the title CoreIR Changes/Fixes for Integration with CGRA-ME CoreIR Changes/Fixes Aug 27, 2018
prefix?=/usr/local
endif

COREDEBUG = True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think should be checked in, seems like a debugging setting for yourself? FYI you can pass this as a flag to make, e.g. make COREDEBUG=1, rather than setting it in the file explicitly


ifdef COREDEBUG
CXXFLAGS += -O0 -g3 -D_GLIBCXX_DEBUG
CXXFLAGS += -O0 -g3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for this removal?

// return false;
// }
// }
// }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you submit the code in the comments as a separate PR? Or you could include it in this PR, but it would be good to include a test case that shows what this fixes.

@leonardt leonardt mentioned this pull request Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants